Conversation
4e05087 to
ccdd06a
Compare
The requirement for ES5 for |
5ce6b39 to
f374a5b
Compare
It would be really nice if we could drop JS_OF_OCAML completely from the CMakeLists file at some point. Are you close to being able to use the defaults? |
We will still need the jsoo export wrapper even after getting the default build settings to work. I was getting somewhere with the defaults, though it seemed like a bit more work was required before we could use them just yet. (hoping to have time over the next month to allocate towards figuring things out). |
Can you explain what this is? Are you referring to
|
|
Should be good to land now. PTAL |
kripken
left a comment
There was a problem hiding this comment.
lgtm % question and test failures
There is now just one emscripten target called `binaryen_js`. The settings for these two targets were almost identical except for. 1. Some JS_OF_OCAML specific stuff which is kept for the new unified target. 2. `-sASSERTIONS` was being forced (See #2507). I dropped this in favor of doing a debug build in testing where this is enabled by default.
| } | ||
| } else { | ||
| e.message = e.message.replace('Fatal:', ''); | ||
| e.message = e.message.trim(); |
There was a problem hiding this comment.
This changes are needed in order to get the test/binaryen.js/errors.js test to pass in debug mode.
We were simply not running any of the emscripten tests in debug build before this change.
There is now just one emscripten target called
binaryen_js. The settings for these two targets were almost identical except for.-sASSERTIONSwas being forced (See Avoid errors in binaryen.js assertions builds, and enable ASSERTIONS in debug builds. #2507). I dropped this in favor of doing a debug build in testing where this is enabled by default.The changes to the post.js files are needed to get the binaryenjs tests to pass in debug mode. We were simply not running any tests in debug mode before this change.